-
Notifications
You must be signed in to change notification settings - Fork 77
Support RAML 1.0 resource types declaration #159
Conversation
Pull Request Test Coverage Report for Build 460
💛 - Coveralls |
Pull Request Test Coverage Report for Build 466
💛 - Coveralls |
c5ed880
to
86364ae
Compare
RAML 1.0 specification declares resource types as array, while 0.8 was array of arrays. |
src/Parser.php
Outdated
$keyedResourceTypes[$i] = $resourceType; | ||
} | ||
} | ||
|
||
foreach ($ramlData['resourceTypes'] as $resourceType) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks odd that $ramlData['resourceTypes']
is processed twice. This doesn't feel right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that, removed unnecessary code.
src/Parser.php
Outdated
$keyedResourceTypes[$k] = $t; | ||
|
||
foreach ($ramlData['resourceTypes'] as $i => $resourceType) { | ||
if (\is_int($i)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Readability can be improved by extracting the if
body in a submethod, eg private function isRaml08(string|int $key): bool
. Indentation levels can also be simplified by replacing the else
consrtuction with continue
as part of the if
.
The final code will read something like:
foreach ($ramlData['resourceTypes'] as $key => $resourceType) {
if ($this->isRaml08($key)) {
... // logic to process 0.8 resource types
continue;
}
... // logic to process 1.0 resource types
}
...
/**
* @param string|int $key
* @return bool
*/
private function isRaml08($key)
{
return \is_int($key);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. To reduce cognitive complexity we can move out this part of code in "getKeydedResourceTypes" method.
As we need exactly one or two cycles on resourceTypes, there are almost nothing to improve in code itself. Maybe recursive getKeyedResourceTypes call will look better but it leads to unnecessary array_merge for v0.8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think a recursive approach is the best one here as there is a known and very low number of iterations. A recursive approach can easily clear the way for otherwise broken, over-nested resources.
fc87b3d
to
2242a0b
Compare
2242a0b
to
000697e
Compare
@rgzp thank you for this contribution and the speedy changes. |
No description provided.